-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Student #205
base: main
Are you sure you want to change the base?
Student #205
Conversation
…now in my own fork instead of Moana's)
…erence frame transformation function
…unction to form this transformation matrix)
…of floats, outputs: numpy arrays (needs to be reviewed in implementation)
…l and magnetic) on actor. started with set_attitude_model and attitude model class as well.
…access the I-matrix)
deleted redundant function (which I added myself)
pointing vector function
Gravity disturbances
Aerodynamic disturbances
Student clean
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Issue #207 shall be taken into account in a separate PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Thanks for the cleaning. A few general comments in addition:
- Please add short examples to the readme on using these models
- Please add tests for reference frame conversions
For the rest see individual comments. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we expose these feature to the users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logging missing in this file
import numpy as np | ||
|
||
|
||
def compute_transformation_matrix_eci_rpy(r, v): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this function is not used else, where so probably should be _compute_transformation_matrix_eci_rpy ?
There is no test for this function
assert np.round(sat1.temperature_in_K, 3) == 278.522 | ||
|
||
|
||
def attitude_and_orbit_test(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong function name will not be run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong filename, has to end with _test or it will not be run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was included by mistake. I removed it.
return sentinel2B, maspalomas_groundstation | ||
|
||
|
||
def main(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will not be run since there is no function test_
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
# Pointing vector from sat2 must not be rotated. | ||
assert np.all(sat2.pointing_vector == np.array([-1.0, 0.0, 0.0])) | ||
# Sat2 angular velocity in the body frame must stay zero: | ||
assert np.all(sat2._attitude_model._actor_angular_velocity == np.array([0.0, 0.0, 0.0])) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only checking against trivial values, no actual check if the model produces correct values after e.g. 5 seconds of rotation
@rasmusmarak this is the work @GabrieleMeoni was mentioning in our call. Maybe, a good first step for you could be to create a small example notebook branching out of this to try out if you can compute the point on Earth's surface that a satelliteActor is pointing at? Ideally any problems and questions that arise during that can also be used in this PR as feedback. (Maybe @GabrieleMeoni could start by just adding a few sentences to the readme on how to use this to enable this?) |
Sounds like a good start, I'll make sure to have a look at some initial notebook example! :-) |
testing to be compliant to PASEOS).
Description
Summary of changes
Resolved Issues
How Has This Been Tested?
Refer to the specific tests included in the PR.
Issue #207 shall be taken into account in a separate PR.